Fix NVMe raw_instance_storage device enumeration for all instance families#196
Conversation
… validation of disk mappings
Update test to expect /dev/nvme2n1 (instance storage) instead of /dev/nvme0n1 (root device). This aligns with the corrected NVMe device enumeration logic: - nvme0n1: root EBS - nvme1n1: ephemeral EBS (when configured) - nvme2n1+: instance storage devices
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
rkoster
requested review from
a team,
KauzClay and
anshrupani
and removed request for
a team
rkoster
moved this from Pending Merge | Prioritized
to Pending Review | Discussion
in Foundational Infrastructure Working Group
Device paths for NVMe raw ephemeral disks now start at nvme0n1 instead of nvme2n1, as the agent performs runtime discovery and the hints are informational only.
|
After discussing this on the community meeting, some people noted that the way we hardcode the NVMe numbering is dangerous since it is not guaranteed that the EBS volumes will be on the same ones every time. |
|
I did not entirely understand what is the issue, so maybe you can just ignore my comment. I am just wondering if this might have been fixed already by cloudfoundry/bosh-linux-stemcell-builder#462 ? At least from the issue description it looks like the exact same issue that I investigated some time ago. |
You might be right here. We did not have a bosh director release since you reverted the PR cloudfoundry/bosh-agent#391. I guess that is still the root cause of this issue ? @neddp @fmoehler |
|
Hi @fmoehler, Thank you for pointing out PR cloudfoundry/bosh-linux-stemcell-builder#462! That PR fixes EBS volume identification (volumes with AWS metadata). This PR addresses instance storage discovery (see #155 for reference), which cannot use the same approach because instance storage volumes have no AWS metadata. The two PRs are complementary. |
|
/easycla |
|
Testing pending. |
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bosh_aws_cpi/lib/cloud/aws/instance_type_info.rb`:
- Around line 24-31: The predicate instance_storage_nvme_naming? narrows NVMe
detection to only nvme_support == 'required' which breaks the contract by
excluding 'supported' types; update the method (instance_storage_nvme_naming?)
to return true when info.instance_storage_info&.nvme_support is either
'required' or 'supported' so BlockDeviceManager will use NVMe (/dev/nvme*n1)
naming for both supported and required instance types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 491cb9d5-b884-4204-9231-d4fe6d6a93a3
📒 Files selected for processing (4)
src/bosh_aws_cpi/lib/cloud/aws/block_device_manager.rbsrc/bosh_aws_cpi/lib/cloud/aws/instance_type_info.rbsrc/bosh_aws_cpi/spec/unit/cloud_v2_spec.rbsrc/bosh_aws_cpi/spec/unit/instance_type_info_spec.rb
💤 Files with no reviewable changes (1)
- src/bosh_aws_cpi/spec/unit/cloud_v2_spec.rb
|
Tested the changes and it is working as expected. Something to note, the new API call requires the |
There was a problem hiding this comment.
Pull request overview
This PR fixes raw_instance_storage boot failures/timeouts on AWS Nitro/NVMe instances by removing reliance on non-deterministic /dev/nvme*n1 ordering and eliminating a stale hardcoded NVMe instance-family allowlist. It introduces runtime instance-type capability detection via the EC2 DescribeInstanceTypes API and updates block device mapping behavior so instance storage on NVMe instances is handled via agent-side discovery.
Changes:
- Add
InstanceTypeInfoto detect (and cache) EBS-via-NVMe and instance-storage NVMe naming usingDescribeInstanceTypes. - Update
BlockDeviceManagerto (a) choose EBS by-id paths when required and (b) filterraw_ephemeralfrom AWS block device mappings on NVMe instance-storage types while still emitting agent “hint” paths. - Update Cloud V1/V2 disk-attach flows and unit tests to pass/use
InstanceTypeInfo.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/bosh_aws_cpi/lib/cloud/aws/instance_type_info.rb | New EC2 DescribeInstanceTypes-backed capability detector with per-process caching and retry logic. |
| src/bosh_aws_cpi/lib/cloud/aws/block_device_manager.rb | Removes hardcoded NVMe family list; uses InstanceTypeInfo for EBS pathing and NVMe instance-storage mapping filtering + hint generation. |
| src/bosh_aws_cpi/lib/cloud/aws/cloud_core.rb | Instantiates and exposes instance_type_info; injects it into BlockDeviceManager. |
| src/bosh_aws_cpi/lib/cloud/aws/cloud_v1.rb | Passes instance_type_info into BlockDeviceManager.device_path call sites. |
| src/bosh_aws_cpi/lib/cloud/aws/cloud_v2.rb | Passes instance_type_info into BlockDeviceManager.device_path during attach. |
| src/bosh_aws_cpi/lib/cloud/aws.rb | Requires the new instance_type_info implementation. |
| src/bosh_aws_cpi/spec/unit/instance_type_info_spec.rb | New unit coverage for InstanceTypeInfo behavior, caching, and error cases. |
| src/bosh_aws_cpi/spec/unit/block_device_manager_spec.rb | Updates specs for injected InstanceTypeInfo and NVMe raw-ephemeral hint behavior. |
| src/bosh_aws_cpi/spec/unit/cloud_v2_spec.rb | Adjusts attach-disk tests to use mocked instance_type_info behavior. |
| src/bosh_aws_cpi/spec/unit/attach_disk_spec.rb | Stubs DescribeInstanceTypes for NVMe-required instance types to validate by-id device paths. |
| src/bosh_aws_cpi/spec/unit/create_stemcell_spec.rb | Updates expectations for the expanded device_path signature. |
| src/bosh_aws_cpi/spec/spec_helper.rb | Adds a default stub for describe_instance_types in the EC2 client mock. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
beyhan
moved this from Pending Merge | Prioritized
to Pending Review | Discussion
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Pending Review | Discussion
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
|
@neddp - I paused the auto-release job since we ant to release this change as either a Minor or Major bump. |
|
Thanks, @aramprice! I can see the Concourse pipelines are failing, because of the missing permission. The user must be modified manually and I have also updated the docs here - /pull/214 |
Problem
The
raw_instance_storagefeature was causing VM boot failures and timeouts on NVMe-based instances (i3, i3en, i4i, c6id, m6id, r6id, etc.) when attempting to use AWS instance storage as raw ephemeral disks.NVMe device enumeration order is non-deterministic on AWS Nitro instances. The kernel discovers NVMe devices based on PCIe enumeration order, which varies between boots and instance types. This means:
/dev/nvme0n1might be the root EBS volume OR instance storageAdditionally, the CPI maintained a hardcoded list of NVMe instance families (
NVME_INSTANCE_FAMILIES) that became stale as AWS added new instance types - requiring a CPI release for every new family.Solution
1. Replace hardcoded NVMe family list with EC2 API detection
Introduced
InstanceTypeInfoclass that queriesDescribeInstanceTypesAPI to determine NVMe characteristics at runtime. Results are cached per CPI process lifetime (~1.4s per unique instance type, subsequent lookups are instant).Two methods cover distinct behaviors:
ebs_requires_nvme_path?- true only for Nitro (nvme_support=required). Used for EBS device path selection (/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*vs/dev/xvd*)instance_storage_nvme_naming?- true for Nitro + i3 family (nvme_support=required|supported). Used for raw ephemeral device handlingThis distinction is critical: i3 is Xen-based (EBS uses
/dev/xvd*paths) but its instance storage IS NVMe (/dev/nvme*n1).2. Filter raw ephemeral from block device mappings on NVMe instances
On NVMe instances, AWS auto-attaches instance storage - it cannot be specified in
BlockDeviceMappings. The CPI now filtersraw_ephemeralentries from the RunInstances API call for NVMe instances, while still passing the correct device count/hints to the agent.3. Generate informational device hints for agent discovery
For NVMe instances, the CPI generates sequential hints (
/dev/nvme0n1,/dev/nvme1n1, etc.). These are informational only - the agent discovers actual instance storage devices at runtime by excluding EBS volumes via/dev/disk/by-id/symlinks.API Performance
DescribeInstanceTypescall: ~1.4sRequestLimitExceeded,InternalError)Related PRs
Potentially fixes #155
This must be merged together with the agent changes - cloudfoundry/bosh-agent#396